Skip to content

Support the Replit Audio messages#1525

Closed
lhchavez wants to merge 2 commits intonovnc:masterfrom
lhchavez:replit-audio-extension
Closed

Support the Replit Audio messages#1525
lhchavez wants to merge 2 commits intonovnc:masterfrom
lhchavez:replit-audio-extension

Conversation

@lhchavez
Copy link
Copy Markdown
Contributor

@lhchavez lhchavez commented Feb 22, 2021

This change builds upon #1457 to support encoded audio support for the
RFB protocol. Notably, it:

a) Adds a button to toggle the audio stream. This is needed since most
browsers require the audio play action to be run in a user event
handler, so having the toggle audio be an explicit button achieves
that (and is also to avoid annoying users).
b) Drops support for the QEMU audio extension and instead uses the
Replit Audio messages, which allows for explicit negotiation for
audio compression.
c) Makes the audio library more robust and now works in Chrome and
Firefox.

The protocol extension and a first implementation can be found at https://github.com/replit/rfbproxy (tested with TigerVNC as the underlying server).

Tested in Chrome and Firefox (missing a volunteer to fix in Safari. maybe @tinyzimmer can help?)

@lhchavez lhchavez force-pushed the replit-audio-extension branch from 8d8da8d to 4bdcdbe Compare February 22, 2021 04:06
Comment thread core/rfb.js Outdated
lhchavez added a commit to lhchavez/rfbproto that referenced this pull request Feb 22, 2021
These messages are intended to allow clients to request the server to
send compressed audio (with the Opus and MP3 codecs) in push and pull
models.

The discussion is in https://groups.google.com/g/novnc/c/pQ6h2xLf3Kc

Reference implementation (client-side): novnc/noVNC#1525
Reference implementation (server-side): https://github.com/replit/rfbproxy

Signed-off-by: Luis Héctor Chávez <luis@repl.it>
tinyzimmer and others added 2 commits March 26, 2021 19:31
This change:

a) Adds a button to toggle the audio stream. This is needed since most
   browsers require the audio play action to be run in a user event
   handler, so having the toggle audio be an explicit button achieves
   that (and is also to avoid annoying users).
b) Drops support for the QEMU audio extension and instead uses the
   Repl.it Audio messages, which allows for explicit negotiation for
   audio compression.
c) Makes the audio library more robust and now works in Chrome and
   Firefox.
@lhchavez lhchavez force-pushed the replit-audio-extension branch from 4bdcdbe to 70c57e8 Compare March 29, 2021 13:19
lhchavez added a commit to replit/noVNC that referenced this pull request Apr 6, 2021
This change:

a) Adds a button to toggle the audio stream. This is needed since most
browsers require the audio play action to be run in a user event
handler, so having the toggle audio be an explicit button achieves
that (and is also to avoid annoying users).
b) Drops support for the QEMU audio extension and instead uses the
Repl.it Audio messages, which allows for explicit negotiation for
audio compression.
c) Makes the audio library more robust and now works in Chrome and
Firefox.

(cherry-pick of novnc#1525)
@CendioOssman
Copy link
Copy Markdown
Member

I'm trying to test this but I'm getting very choppy audio. Any suggestions on debugging and getting it working?

@lhchavez
Copy link
Copy Markdown
Contributor Author

lhchavez commented Apr 8, 2021

I'm trying to test this but I'm getting very choppy audio. Any suggestions on debugging and getting it working?

oh no! does the rfbproxy log mention a lot of reconnects? if not, you can add this diff to diagnose it client-side:

diff --git a/core/util/audio.js b/core/util/audio.js
index dbfc400..40b37c6 100644
--- a/core/util/audio.js
+++ b/core/util/audio.js
@@ -125,6 +125,12 @@ export default class AudioStream {
     // seeked to the latest possible position that doesn't trigger buffering
     // to avoid an arbitrarily large desync between video and audio.
     _appendChunk(timestamp, chunk) {
+        console.debug("appending chunk", {
+            timestamp,
+            currentTime: this._audio.currentTime,
+            buffered: this._audio.buffered.length && this._audio.buffered.end(this._audio.buffered.length - 1),
+        });
+
         this._audioBuffer.appendBuffer(chunk);
         if (
             timestamp - this._audio.currentTime > MAX_ALLOWABLE_DESYNC &&

are both connections local, or are you using the PoC that i shared before?

lhchavez added a commit to lhchavez/rfbproto that referenced this pull request Apr 22, 2021
These messages are intended to allow clients to request the server to
send compressed audio (with the Opus and MP3 codecs) in push model.

The discussion is in https://groups.google.com/g/novnc/c/pQ6h2xLf3Kc

Reference implementation (client-side): novnc/noVNC#1525
Reference implementation (server-side): https://github.com/replit/rfbproxy

Signed-off-by: Luis Héctor Chávez <luis@repl.it>
@CendioOssman
Copy link
Copy Markdown
Member

Sorry about the delay. I could spend a little time on this today, but unfortunately not a lot. The proxy doesn't log anything so I'm afraid there is no help there.

Without any modifications I'm getting this in the browser log:

maximum allowable desync reached 
Object { readyState: 2, buffered: "0.08", seekable: "0.08", time: "0.08", delta: "1.97" }
audio.js:133:21
maximum allowable desync reached 
Object { readyState: 2, buffered: "6.10", seekable: "6.10", time: "4.13", delta: "2.01" }
audio.js:133:21
maximum allowable desync reached 
Object { readyState: 2, buffered: "10.19", seekable: "12.30", time: "8.22", delta: "2.04" }
audio.js:133:21
maximum allowable desync reached 
Object { readyState: 2, buffered: "14.30", seekable: "14.30", time: "12.34", delta: "2.00" }
audio.js:133:21
...

With that patch I'm getting a bunch of lines, but nothing obviously wrong:

...
appending chunk 
Object { timestamp: 25.521, currentTime: 25.15025, buffered: 25.521 }
audio.js:128:17
appending chunk 
Object { timestamp: 25.561, currentTime: 25.15025, buffered: 25.561 }
audio.js:128:17
appending chunk 
Object { timestamp: 25.601, currentTime: 25.231, buffered: 25.601 }
audio.js:128:17
appending chunk 
Object { timestamp: 25.641, currentTime: 25.231, buffered: 25.641 }
audio.js:128:17
appending chunk 
Object { timestamp: 25.721, currentTime: 25.351937, buffered: 25.721 }
audio.js:128:17
appending chunk 
Object { timestamp: 25.761, currentTime: 25.351937, buffered: 25.761 }
audio.js:128:17
...

Whole log: console-export-2021-7-23_15-0-12.txt

@lhchavez
Copy link
Copy Markdown
Contributor Author

Sorry about the delay. I could spend a little time on this today, but unfortunately not a lot. The proxy doesn't log anything so I'm afraid there is no help there.

Without any modifications I'm getting this in the browser log:

maximum allowable desync reached 
Object { readyState: 2, buffered: "0.08", seekable: "0.08", time: "0.08", delta: "1.97" }
audio.js:133:21
maximum allowable desync reached 
Object { readyState: 2, buffered: "6.10", seekable: "6.10", time: "4.13", delta: "2.01" }
audio.js:133:21
maximum allowable desync reached 
Object { readyState: 2, buffered: "10.19", seekable: "12.30", time: "8.22", delta: "2.04" }
audio.js:133:21
maximum allowable desync reached 
Object { readyState: 2, buffered: "14.30", seekable: "14.30", time: "12.34", delta: "2.00" }
audio.js:133:21
...

With that patch I'm getting a bunch of lines, but nothing obviously wrong:

...
appending chunk 
Object { timestamp: 25.521, currentTime: 25.15025, buffered: 25.521 }
audio.js:128:17
appending chunk 
Object { timestamp: 25.561, currentTime: 25.15025, buffered: 25.561 }
audio.js:128:17
appending chunk 
Object { timestamp: 25.601, currentTime: 25.231, buffered: 25.601 }
audio.js:128:17
appending chunk 
Object { timestamp: 25.641, currentTime: 25.231, buffered: 25.641 }
audio.js:128:17
appending chunk 
Object { timestamp: 25.721, currentTime: 25.351937, buffered: 25.721 }
audio.js:128:17
appending chunk 
Object { timestamp: 25.761, currentTime: 25.351937, buffered: 25.761 }
audio.js:128:17
...

Whole log: console-export-2021-7-23_15-0-12.txt

gotcha. let me build a Docker container so that we're working with the same environment, and have all the logging.

@CendioOssman
Copy link
Copy Markdown
Member

@lhchavez, were you able to get a docker image up for testing?

@samhed samhed added this to the Future Features milestone Aug 26, 2021
@lhchavez
Copy link
Copy Markdown
Contributor Author

not yet, i've been on vacation for a couple of weeks. i'll give it a try tomorrow!

@lhchavez
Copy link
Copy Markdown
Contributor Author

lhchavez commented Sep 3, 2021

not yet, i've been on vacation for a couple of weeks. i'll give it a try tomorrow!

got it:

FROM ubuntu:groovy

RUN DEBIAN_FRONTEND=noninteractive apt-get update -y && \
    DEBIAN_FRONTEND=noninteractive TZ=UTC apt-get install \
      --no-install-recommends -y \
      curl \
      fluxbox \
      gpg \
      gpg-agent \
      libmp3lame0 \
      libopus0 \
      libpulse0 \
      mplayer \
      pulseaudio \
      software-properties-common \
      tigervnc-standalone-server \
      vlc \
      xterm \
      xz-utils && \
    rm -rf /var/lib/apt/lists/*

RUN curl -sL https://github.com/replit/rfbproxy/releases/download/v0.1.1-b81eb2f/rfbproxy.tar.xz | tar xJ -C /

RUN useradd --create-home runner
WORKDIR /home/runner
USER runner

ENV DISPLAY=:0
# Verbosity can be upped by changing `RUST_LOG=debug` in the script below.
RUN echo $'\n\
#!/bin/bash\n\
Xtigervnc --SecurityTypes=None --rfbport=5901 --localhost "${DISPLAY}" 2>&1 | sed "s/^/[Xtigervnc ] /" & \n\
sleep 1 \n\
RUST_LOG=info rfbproxy --enable-audio 2>&1 | sed "s/^/[rfbproxy  ] /" & \n\
sleep 1 \n\
fluxbox 2>&1 | sed "s/^/[fluxbox   ] /" & \n\
pulseaudio --load="module-null-sink sink_name=null" 2>&1 | sed "s/^/[pulseaudio] /" & \n\
sleep 1 \n\
/usr/bin/vlc --no-qt-privacy-ask --alsa-audio-device=pulse --aout=pulse http://ice2.somafm.com/groovesalad-128-aac 2>&1 | sed "s/^/[vlc       ] /"'\
> /home/runner/start.sh

EXPOSE 5900
ENTRYPOINT ["/bin/bash", "/home/runner/start.sh"]

run with:

docker build -f Dockerfile -t rfbproxy . && docker run -it --rm -p 5900:5900 rfbproxy

to get some nice tunes courtesy of https://somafm.com/groovesalad/ on ws://localhost:5900/

@CendioOssman
Copy link
Copy Markdown
Member

Thanks. That image seems to work fine as constructed. I get a noticeable latency in the audio though, but that is somewhat expected given that this protocol has no synchronisation.

However, only VLC seems to work properly. If I use mplayer then the stuttering is back. Can you see if it's the same when you test it? Are things perhaps reliant on the application providing a certain amount of buffered data?

@Neustradamus
Copy link
Copy Markdown

@lhchavez: Any news about it?

@masad-frost
Copy link
Copy Markdown

Hey! I'm from Replit and would love to land this instead of maintaining our own fork.

What would you like to see for this PR to land?

For what it's worth, we have been using this in production for a few months now and people seem to be happy with the audio.

@masad-frost masad-frost mentioned this pull request Oct 27, 2022
@CendioOssman
Copy link
Copy Markdown
Member

Please see my previous comment. I could not get this to work reliably, which means I don't feel confident it will work reliably for our users.

Have you applied any more fixes in your fork that aren't included here?

@CendioOssman
Copy link
Copy Markdown
Member

No response for two years. Closing.

@Neustradamus
Copy link
Copy Markdown

Neustradamus commented Nov 20, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants